fix(sql): support unsigned ASOF join keys#19603
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b940b91509
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
sundy-li
left a comment
There was a problem hiding this comment.
Found a blocking issue in the unsigned ASOF sentinel handling. Using the type-domain max/min as infinity() / ninfinity() makes the rewrite drop valid boundary matches (>= at MAX and <= at MIN). See the inline comment for a concrete example.
sundy-li
left a comment
There was a problem hiding this comment.
Focused verification passed, but the new unsigned sentinels still make ASOF results wrong at the legal type boundaries. rewrite_asof() uses infinity()/ninfinity() as the LEAD default and then applies strict < / > bounds, so rows at UInt*_MAX or UInt*_MIN cannot match the terminal or initial interval even though they should. This means unsigned ASOF joins are still incorrect for valid key values and need a different unbounded strategy or dedicated operator before this lands.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48881149de
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
unwrap()calls withBadDataValueTypepropagation so unsupported types return an error instead of panickingFixes #19570
Tests
Validation:
cargo test -p databend-common-expression --test it test_integer_infinity_boundariescargo clippy -p databend-common-sql --lib --tests -- -D warningsType of change
This change is